Skip to content

feat(fill): pass blobSchedule into t8n binary #2264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat(fill): pass blobSchedule into t8n binary #2264

wants to merge 3 commits into from

Conversation

pdobacz
Copy link
Contributor

@pdobacz pdobacz commented Oct 4, 2025 •
edited
Loading

🗒️ Description

The blobSchedule.json is required by evmone-t8n tool now, in order to fill tests correctly. We add a new structure intended to be used as a JSON generator for the filesystem evaluation inputs - I guess trying to mimic what TransitionToolRequest does, lmk if I misread the idea here.

Appropriate change on the evmone end: ipsilon/evmone#1330

🔗 Related Issues or PRs

N/A.

✅ Checklist

  • All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks: 
    uvx --with=tox-uv tox -e lint,typecheck,spellcheck,markdownlint
  • All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
  • All: Considered adding an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.
  • All: Set appropriate labels for the changes (only maintainers can apply labels).

@pdobacz pdobacz mentioned this pull request Oct 4, 2025
Merged
@pdobacz pdobacz force-pushed the feat/t8n-filesystem-blob-schedule branch from cbc03ea to 220b7cf Compare October 4, 2025 09:37
@masoudahg00 masoudahg00 linked an issue Oct 4, 2025 that may be closed by this pull request
a5ec90e29270463518de716b1aa6e79c864491203d1dfe6b35ee79499805bd2145296f3a925240aa "My portfolio id" : "35VmgKWofZxWowQ7O6CL8K"# !/usr/bin/env ruby require_relative '../compiler/base' generated = Compiler.new('test.src').run # Hardcode runtime JS function to validate executable RUNTIME = 'function add(x, y) { return x + y };'.freeze TEST = 'console.log(f(1,2))'.freeze puts [RUNTIME, generated, TEST].join("\n") masoudahg00/Leyli-Mahmudi#2
Open
@pdobacz pdobacz force-pushed the feat/t8n-filesystem-blob-schedule branch from 220b7cf to 030733b Compare October 6, 2025 08:11
@pdobacz pdobacz marked this pull request as draft October 6, 2025 09:01
@pdobacz
Copy link
Contributor Author

pdobacz commented Oct 6, 2025

coverted to draft b/c maybe we should send more in that JSON (to include network and (future) ...Time fields to be able to t8n in e.g. BPO1)

@pdobacz pdobacz marked this pull request as ready for review October 7, 2025 08:13
@pdobacz
Copy link
Contributor Author

pdobacz commented Oct 7, 2025

Ready to review. The version pushed now will process (in line with ipsilon/evmone#1330) an --input.config argument and JSON, so that end-game schedule (with ...Time) fields is support-able, doesn't worry about handling fill --fork OsakaToBPO1AtTime15k, for now. AFAIK this doesn't break any existing fill-capabilities of evmone, so we can maybe think about providing them as a 2nd step

@spencer-tb spencer-tb removed a link to an issue Oct 7, 2025
a5ec90e29270463518de716b1aa6e79c864491203d1dfe6b35ee79499805bd2145296f3a925240aa "My portfolio id" : "35VmgKWofZxWowQ7O6CL8K"# !/usr/bin/env ruby require_relative '../compiler/base' generated = Compiler.new('test.src').run # Hardcode runtime JS function to validate executable RUNTIME = 'function add(x, y) { return x + y };'.freeze TEST = 'console.log(f(1,2))'.freeze puts [RUNTIME, generated, TEST].join("\n") masoudahg00/Leyli-Mahmudi#2
Open
marioevz
marioevz reviewed Oct 7, 2025
View reviewed changes
Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for implementing this, some comments.

src/ethereum_clis/cli_types.py Outdated

fork: Fork = Field(..., alias="network")
chain_id: int = Field(..., alias="chainid")
blob_schedule: BlobSchedule
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in chat group, let's pass instead:

Suggested change
blob_schedule: BlobSchedule
blob_params: BlobParams

Also, we already have a definition that fits our needs in here:

execution-spec-tests/src/ethereum_test_base_types/composite_types.py

Lines 487 to 492 in 56ea9f2

class ForkBlobSchedule(CamelModel):
"""Representation of the blob schedule of a given fork."""
target_blobs_per_block: HexNumber = Field(..., alias="target")
max_blobs_per_block: HexNumber = Field(..., alias="max")
base_fee_update_fraction: HexNumber = Field(...)

We could even rename that type to BlobParams because the current name ForkBlobSchedule doesn't make much sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pointers. I've pushed a commit which I think follows these suggestions, it seems to work fine with the (revised) evmone from the respective PR. I wasn't exactly sure how to work in the fork and blob params resolution, so please take a look at the recent commit.

I haven't yet done the renaming of ForkBlobSchedule to not obscure the PR. Can do as a new commit after, or in a separate PR (preferably).

@pdobacz pdobacz force-pushed the feat/t8n-filesystem-blob-schedule branch from 47abba4 to 113ff32 Compare October 9, 2025 21:15
@pdobacz pdobacz force-pushed the feat/t8n-filesystem-blob-schedule branch from 113ff32 to 5decc1d Compare October 10, 2025 09:56
@pdobacz pdobacz requested a review from marioevz October 10, 2025 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@marioevz marioevz Awaiting requested review from marioevz

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pdobacz @marioevz